LGTM — no blockers. Nicely-scoped robustness fix, and it directly closes the "handle all agents like skills add / gstack" concern raised on #1738. Verified independently:

1. Upstream flags valid at the pinned vercel-labs/skills@v1.5.13 — -a, --agent, -s, --skill, -y, --yes all exist (src/cli.ts), and --all is literally shorthand for --skill '*' --agent '*' -y. So replacing --all with --skill '*' --agent <resolved> --yes is a correct narrowing, not a behavior guess.
2. resolveAgentTargets never resolves to empty — existing project skill folders (honor intent) → CLAUDECODE → PATH-probe of installed agent CLIs (the gstack approach) → floor (claude-code + .agents universal). The floor fallback is the key safety net so a blank machine still gets a sane target. DIR_TO_KEY keys verified against v1.5.13; the explicit dir↔key mapping (.factory↔droid, .hermes↔hermes-agent) and the OpenClaw exclusion (bare skills/ collision) are thoughtful.
3. Composes correctly with #1744's --copy — runSkillsAdd still appends --copy after the resolved args; explicit extraArgs still wins for callers/tests; the prior installAllSkills({extraArgs:['--all','--yes']}) caller is updated to use resolution.

Good coverage (skillsTargets.test.ts +130 + skills.test.ts), and the Installing to: … log is nice transparency.

— Jerrai

Review — fix(cli): scope skill installs to relevant agents

SHA: 7d28f29
Verdict: LGTM — clean, well-reasoned change. A few observations below, nothing blocking.

What this does (my read)

The PR replaces every --all (= --skill '*' --agent '*' -y) occurrence in the skills install pipeline with a resolver (resolveAgentTargets) that picks only the agents the project or machine actually uses. The resolver lives in a new skillsTargets.ts utility with a clear priority chain: existing project folders > CLAUDECODE env > PATH-detected CLIs > a claude-code + universal floor. The buildSkillsAddArgs helper produces --skill '*' --agent <targets...> --yes — verified against the upstream parseAddOptions which consumes variadic args after --agent until it hits another flag.

Correctness

1. DIR_TO_KEY mapping — verified against vercel-labs/skills@v1.5.13 agents.ts: .claude -> claude-code, .agents -> universal, .hermes -> hermes-agent, .factory -> droid, .kiro -> kiro-cli. All correct.

2. Upstream --agent parsing — the upstream parseAddOptions grabs variadic non-flag args after --agent, so --agent claude-code universal --yes parses as agent: ["claude-code", "universal"], yes: true. The generated arg order from buildSkillsAddArgs is correct.

3. init.ts cleanup — the old code had a CLAUDECODE branch that passed ["--all", "--agent", "claude-code", "--yes"] and a fallback of ["--all", "--yes"]. The new code delegates entirely to resolveAgentTargets, which handles the CLAUDECODE case (step 2a) and applies the same logic init was hand-rolling. Good simplification — the special-casing was duplicated between init and skills, now it's in one place.

4. skills update path — previously passed ["--all", "--yes"] as extraArgs, now passes no extraArgs so runSkillsAdd resolves targets itself. The installAllSkills → runSkillsAdd chain correctly falls through to resolveAgentTargets when opts.extraArgs is undefined. Clean.

5. extraArgs escape hatch — runSkillsAdd still respects an explicit extraArgs when passed, so callers that need specific targeting can bypass the resolver. Good for forward compatibility.

Observations (non-blocking)

1. isDir is slightly over-cautious — it calls both existsSync and statSync, but statSync already throws ENOENT when the path doesn't exist, so the existsSync check is redundant. The try/catch around both handles it fine though, so this is just a nit — no functional impact.

2. PATH probe creates empty files as "executables" in the test helper (pathWith) — works on Linux/macOS because existsSync doesn't check the execute bit. On Windows the test also probes .exe/.cmd/.bat extensions, but the test helper doesn't create those suffixed files. This is fine because the tests run on Linux and the Windows path is an unlikely CI target, but worth noting if you ever add Windows test matrix.

3. The DETECTABLE list is conservative — only 7 CLIs are probed. Agents like goose, continue, kilo, devin, etc. are omitted. This seems intentional (the comment says "the gstack approach"), and the universal floor covers agents that read .agents/skills anyway. Just flagging for awareness — if a user has only goose installed, they'd get the claude-code + universal floor rather than the goose key. Since goose uses .goose/skills (not .agents/skills), the universal bucket wouldn't reach it. This is an edge case and arguably the right call for now — expanding the list can be a follow-up.

4. keysForDirs iteration order — relies on Object.entries(DIR_TO_KEY) preserving insertion order (spec-guaranteed for string keys in V8/modern engines). The comment "claude-code first" in the test matches the object literal order. Solid.

5. Test coverage — good coverage of all resolver branches: existing folders, dir-to-key mapping, CLAUDECODE env, PATH detection, universal-bucket collapse, floor fallback, the "never *" invariant, and buildSkillsAddArgs shape. The skills.test.ts additions pin the spawn args and add a "never --all" regression guard. The mock of resolveAgentTargets in skills.test.ts is the right call — isolates command-arg construction from environment-dependent resolution.

CI

Several checks still pending/skipping at time of review (Typecheck, Build, CLI smoke, Test, etc.). No failures observed.

Overall this is a well-structured change with clear reasoning, good test coverage, and correct integration with the upstream CLI. Ship it.

— Miga

Reviewed at 7d28f291. No blockers.

What I verified:

• packages/cli/src/commands/skills.ts:57 keeps runSkillsAdd as the single install chokepoint and replaces the default --all spray with concrete --skill "*" --agent <resolved> --yes args while preserving explicit extraArgs for callers that intentionally bypass resolution.
• packages/cli/src/utils/skillsTargets.ts:128 resolves targets in the right order: existing project skill folders first, then CLAUDECODE, then PATH-detected installed agent CLIs, then the claude-code + universal floor, so it never emits the * wildcard or an empty target set.
• Upstream vercel-labs/skills@v1.5.13 accepts the generated argv shape: parseAddOptions consumes multiple non-flag values after --agent, and the mapped keys (claude-code, universal, hermes-agent, droid, kiro-cli) match upstream agents.ts.
• packages/cli/src/utils/skillsTargets.test.ts:33 plus packages/cli/src/commands/skills.test.ts:200 cover the resolver branches and pin the no---all / no-wildcard regression.

Additive to Rames + Miga: I do not see a blocking gap. Their notes on isDir, the conservative detectable CLI list, and Windows-suffixed test helpers are non-blocking.

CI note: all completed checks were green at review time; Tests on windows-latest was still pending, so merge should wait for that check if required.

Verdict: APPROVE
Reasoning: This narrows the install target set without changing the upstream install contract, and the resolver has a safe floor plus focused tests for the no-spray behavior.

— Magi